Skip to content

Conversation

@gave92
Copy link
Member

@gave92 gave92 commented Nov 1, 2023

Resolved / Related Issues

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Closes Feature: Add terminal Integration #6235

Validation
How did you test these changes?

  • Did you build the app and test your changes?

Screenshots (optional)
image

@files-community files-community locked and limited conversation to collaborators Nov 1, 2023
@yaira2 yaira2 changed the title [Post V3] Terminal Integration Feature: Added terminal Integration Nov 5, 2023
@yaira2 yaira2 changed the title Feature: Added terminal Integration Feature: Added terminal integration Nov 5, 2023
@files-community files-community unlocked this conversation Nov 8, 2023
@yaira2 yaira2 force-pushed the main branch 3 times, most recently from cecddda to 7f8c927 Compare November 17, 2023 16:57
@hez2010
Copy link
Member

hez2010 commented Mar 4, 2024

I'm wondering if we can use the conhost from Windows Terminal instead of using the WebView-based terminal?

@gave92
Copy link
Member Author

gave92 commented Mar 4, 2024

I'd like to but it's huge and I could not find an easy way to integrate it.

@yaira2 yaira2 force-pushed the main branch 2 times, most recently from edf33f7 to c79b2cb Compare March 10, 2024 02:11
@yaira2 yaira2 force-pushed the main branch 3 times, most recently from 32de19b to ba56951 Compare April 4, 2024 21:47
@0x5bfa
Copy link
Member

0x5bfa commented Apr 13, 2024

What's left to do?
Looks solid!

@gave92 gave92 marked this pull request as ready for review April 13, 2024 16:57
@gave92
Copy link
Member Author

gave92 commented Apr 13, 2024

Hey @0x5bfa the TODOs I have in mind are:

  • Add setting to enable/disable the feature
  • Decide if terminal is global or if each tab has its own instance
  • Add setting to store the preferred terminal
  • Define different terminal themes for light/dark mode
  • Remember terminal size and if it was opened or closed when restarting Files

Copy link
Contributor

@gumbarros gumbarros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats for the PR, looking forward to use this feature

@0x5bfa
Copy link
Member

0x5bfa commented Apr 22, 2024

@gave92 Thank you for using CsWin32! This is our short term goal, which is to use source generator everywhere.

@gave92
Copy link
Member Author

gave92 commented May 1, 2024

I've added a setting under Experimental to enable/disable the feature (default: off). The feature may be good enough for first release :)

@gave92 gave92 added the ready for review Pull requests that are ready for review label May 1, 2024
@yaira2 yaira2 changed the title Feature: Added terminal integration Feature: Added experimental terminal integration May 1, 2024
@yaira2
Copy link
Member

yaira2 commented May 1, 2024

I had some notes on this but I'll have another look since it's been a while.

@yaira2
Copy link
Member

yaira2 commented May 1, 2024

The main point to figure out before merging is whether the terminal is global, or if there can be a separate terminal for each tab & pane. I lean towards the second option (similar to VS Code).

@gave92 gave92 marked this pull request as draft May 2, 2024 06:56
@gave92
Copy link
Member Author

gave92 commented May 2, 2024

Holding off while @hez2010 checks if it's possible to use Windows Terminal control.

@0x5bfa 0x5bfa removed the ready for review Pull requests that are ready for review label May 2, 2024
@gave92
Copy link
Member Author

gave92 commented May 8, 2024

Added ability to have more than one terminal 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Pull requests that are ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Add terminal Integration

7 participants